Conversation
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f7e067cd5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const backendResolved = runtime.backend.resolveInstallSource | ||
| ? await runtime.backend.resolveInstallSource(context, localResolved.source) | ||
| : localResolved.source; |
There was a problem hiding this comment.
If backend.resolveInstallSource throws, the temporary file/materialization created by resolveLocalInstallSource is leaked because cleanup is only returned on success and only executed later in runInstallCommand's finally. This is reproducible with source.kind === 'uploadedArtifact' when the backend rejects or times out during source resolution, and it can accumulate temp artifacts across failed installs. Wrap backend source resolution in a local try/finally (or catch + rethrow) so localResolved.cleanup runs on this error path too.
Useful? React with 👍 / 👎.
thymikee
changed the title
thymikee
changed the title
1 participant
Summary
Moves phases 4/5/6 onto the runtime/router surface and trims back the unfinished phase 7 exposure:
admin.*, routerbatch,record,trace, anddiagnostics.*publicreplayandtestfrom the public runtime types and dispatch path, and marks them planned again incommandCatalogadmin.install/admin.reinstalltemp-input cleanup so uploaded-artifact materialization is released even when backend source resolution failsValidation
pnpm formatpnpm typecheckpnpm vitest run src/__tests__/runtime-admin-router.test.tspnpm vitest run src/__tests__/runtime-admin-router.test.ts src/__tests__/runtime-diagnostics-router.test.ts src/__tests__/runtime-public.test.tspnpm check:quickSTATE_DIR=$(mktemp -d); AGENT_DEVICE_STATE_DIR="$STATE_DIR" pnpm check:unit; STATUS=$?; rm -rf "$STATE_DIR"; exit $STATUSpnpm buildSTATE_DIR=$(mktemp -d); AGENT_DEVICE_STATE_DIR="$STATE_DIR" pnpm test:integration; STATUS=$?; rm -rf "$STATE_DIR"; exit $STATUSAGENT_DEVICE_STATE_DIR=$(mktemp -d /tmp/agent-device-e2e-ios-XXXXXX) pnpm test:replay:ios(6 passed)AGENT_DEVICE_STATE_DIR=$(mktemp -d /tmp/agent-device-e2e-ios-device-XXXXXX) pnpm test:replay:ios-device(1 passed)AGENT_DEVICE_STATE_DIR=$(mktemp -d /tmp/agent-device-e2e-macos-rerun-XXXXXX) pnpm test:replay:macos(1 passed on rerun; first attempt hit transient_LSOpenURLsWithCompletionHandler ... error -600launching System Settings)AGENT_DEVICE_STATE_DIR=$(mktemp -d /tmp/agent-device-e2e-android-XXXXXX) pnpm test:replay:androidfailed because no Android devices were connectedAGENT_DEVICE_STATE_DIR=$(mktemp -d /tmp/agent-device-e2e-linux-XXXXXX) pnpm test:replay:linuxfailed because no Linux desktop target was available on this hostAGENT_DEVICE_STATE_DIR=$(mktemp -d /tmp/agent-device-recording-e2e-XXXXXX) AGENT_DEVICE_RECORDING_E2E=1 node --test test/integration/recording-overlay.test.tsexposed existing live failures: iOS tap passed, iOS scroll overlay saw 0 overlay pixels, iOS back-swipe reportediOS runner session restarted during recording, and Android recording cases failed because no Android device was available